-
Notifications
You must be signed in to change notification settings - Fork 545
feat: treat void as null for null coalescing operator #2494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.
b885d99 to
28888d3
Compare
Using void in a union is completely invalid and when used in a native type even crashes PHP: https://3v4l.org/7iULB
Instead of array|void instead array|null should be used.
That said, I'd accept a PR that changes array|void into array|null as a result of FuncCall/StaticCall/MethodCall. So the change needs to happen in MutatingScope::resolveType. It's not enough to change a single rule.
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
28888d3 to
cdaa291
Compare
Thanks for your prompt review 👍
I will take a look.
Looks like you are aware of it already @kesselb , but just for the sake of completeness and issue linking - we discussed this also in szepeviktor/phpstan-wordpress#176, and yeah, WordPress types are wrong there IMO
Oh and phpstan/phpstan#9185 too
Superseded by #2778
Hi 👋
WordPress often uses type definitions like
array|void(e.g. https://github.com/WordPress/wordpress-develop/blob/68be569925ad2a88c97e1308b8a67a57bf17a14f/src/wp-includes/post.php#L2627).$custom_keys = get_post_custom_keys( $post_id ) ?? [];Shows: Expression on left side of ?? is not nullable.